fix(cli): prompt for folder trust before auth#27845
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
📊 PR Size: size/L
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the security and configuration lifecycle of the Gemini CLI by enforcing a folder trust check during the initial startup phase. By prompting users to trust their workspace folder before authentication, the CLI ensures that subsequent operations, such as loading workspace settings or MCP servers, are performed in a secure and consistent environment. The implementation includes a robust relaunch mechanism to apply trust decisions immediately and provides focused tests to validate the startup ordering. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a folder trust prompt mechanism (maybePromptForFolderTrust) to resolve unknown workspace trust states during CLI startup, ensuring it runs before slow operations like authentication. It also adds comprehensive unit tests for this flow. The review feedback suggests a high-severity improvement: when using the prompts library, both stdin and stdout streams from createWorkingStdio() should be passed as the second argument (options) to ensure interactive prompts correctly receive user input when patchStdio() is active.
| const { stdout } = createWorkingStdio(); | ||
| const response = await prompts({ | ||
| type: 'select', | ||
| name: 'trustLevel', | ||
| message: 'Do you want to trust this folder?', | ||
| choices: [ | ||
| { | ||
| title: `Trust folder (${dirName})`, | ||
| value: TrustLevel.TRUST_FOLDER, | ||
| }, | ||
| { | ||
| title: `Trust parent folder (${parentFolder})`, | ||
| value: TrustLevel.TRUST_PARENT, | ||
| }, | ||
| { | ||
| title: "Don't trust", | ||
| value: TrustLevel.DO_NOT_TRUST, | ||
| }, | ||
| ], | ||
| initial: 0, | ||
| stdout, | ||
| }); |
There was a problem hiding this comment.
When patchStdio() is active, process.stdin might be patched or intercepted. To ensure the interactive prompt correctly receives user input, you should pass the working stdin stream (obtained from createWorkingStdio()) to prompts as part of its options (the second argument). Passing stdout in the options object is also the standard and documented way to configure custom streams in prompts.
const { stdin, stdout } = createWorkingStdio();
const response = await prompts({
type: 'select',
name: 'trustLevel',
message: 'Do you want to trust this folder?',
choices: [
{
title: 'Trust folder (' + dirName + ')',
value: TrustLevel.TRUST_FOLDER,
},
{
title: 'Trust parent folder (' + parentFolder + ')',
value: TrustLevel.TRUST_PARENT,
},
{
title: "Don't trust",
value: TrustLevel.DO_NOT_TRUST,
},
],
initial: 0,
}, {
stdin,
stdout,
});There was a problem hiding this comment.
Updated in 0273154. The prompt now explicitly passes stdin: process.stdin and the real stdout from createWorkingStdio(), with a test assertion covering both streams.
I kept the streams on the prompt object rather than the second prompts argument because this repository currently uses prompts@2.4.2; its implementation and TypeScript types pass stdin/stdout through the question object, while the second argument only handles submit/cancel callbacks. Also, createWorkingStdio() currently returns stdout/stderr proxies, not stdin, and patchStdio() only patches stdout/stderr.
0150eb2 to
0273154
Compare
|
Updated in 0273154. I made the prompt explicitly pass both streams: |
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
Fixes #27844
Summary
refreshAuth().Test plan
npm run test --workspace @google/gemini-cli -- folderTrustPrompt.test.ts gemini.test.tsx --testNamePattern "maybePromptForFolderTrust|should resolve folder trust before refreshing auth"npm run typecheck --workspace @google/gemini-clinpm run lint --workspace @google/gemini-cliGEMINI_CLI_HOMEand temporary workspace usingpackages/cli/dist/index.js; the folder trust prompt appeared before auth/UI startup (PROMPT_MS=1609).